Skip to content

Fix ObjectDisposedException in BuildsWhileBuildIsRunningOnServer test#13316

Merged
rainersigwald merged 5 commits intomainfrom
copilot/fix-disposal-order-issue
Mar 3, 2026
Merged

Fix ObjectDisposedException in BuildsWhileBuildIsRunningOnServer test#13316
rainersigwald merged 5 commits intomainfrom
copilot/fix-disposal-order-issue

Conversation

Copy link
Contributor

Copilot AI commented Mar 2, 2026

FileSystemWatcher callbacks could call mre.Set() after mre was already disposed, crashing the test host with ObjectDisposedException. With using var, disposal happens in reverse declaration order — watcher was declared first, so mre was disposed before watcher stopped delivering events.

Changes Made

  • Swap declaration order of mre and watcher in BuildsWhileBuildIsRunningOnServer so watcher is disposed first (silencing callbacks) before mre is disposed:
// Before — mre disposed before watcher:
using var watcher = new System.IO.FileSystemWatcher(dir!);
using ManualResetEvent mre = new ManualResetEvent(false);

// After — watcher disposed first, mre safe:
using ManualResetEvent mre = new ManualResetEvent(false);
using var watcher = new System.IO.FileSystemWatcher(dir!);
  • Added a comment above the declarations explaining the required ordering, so future readers do not accidentally reorder them.

Testing

Existing test BuildsWhileBuildIsRunningOnServer covers the scenario. No new tests needed — this is a correctness fix for the test itself.

Notes

No try/catch or manual EnableRaisingEvents = false needed; correct disposal ordering is sufficient.

Original prompt

Should the declaration order between mre and watcher be changed? If so does that mean the other changes aren't necessary?

[Chronological Review: - User reported a CI crash in Azure DevOps build `#13175` (`buildId=1316368`) with an unhandled `System.ObjectDisposedException` in `MSBuildServer_Tests.cs:line 198` during `FileSystemWatcher` callback execution. - User repeated the same request after a model switch notice, asking how to harden the test and fix the root cause. - Agent inspected `src/MSBuild.UnitTests/MSBuildServer_Tests.cs` and identified disposal-order race in `BuildsWhileBuildIsRunningOnServer`: `using var watcher` declared before `using ManualResetEvent mre`; reverse disposal means `mre` can be disposed before late watcher callbacks run. - Agent edited the test to catch `ObjectDisposedException` around `mre.Set()` and added `watcher.EnableRaisingEvents = false;` before cleanup. - Agent verified updated method lines 176–240. - User then requested: “put up a draft PR”. - Agent began git workflow (status/branch checks, then branch+commit command), but the final branch/commit tool execution was interrupted before completion.]

[Intent Mapping:

  • Explicit request 1: “Ideally we would harden the test so we can clearly isolate what went wrong and get full test results back, and also fix whatever caused this problem. What can we do?”
  • Explicit request 2: “put up a draft PR”
  • Implicit expectations: avoid test-host crash, preserve full test results, implement robust cross-platform handling for asynchronous file events, and package changes into a PR-ready commit.]

[Technical Inventory:

  • Language/runtime: C#/.NET test code.
  • Test framework: xUnit ([Fact], [Theory]), assertions via Shouldly.
  • Components involved: System.IO.FileSystemWatcher, ManualResetEvent, asynchronous Task.Run, process management (Process.GetProcessById, KillTree), MSBuild test infrastructure (TestEnvironment, RunnerUtilities.ExecMSBuild).
  • Platform-specific behavior: macOS FSEvents asynchronous/late callbacks.
  • Repo tooling: git commands via PowerShell for branching/committing.]

[Code Archaeology:

  • File inspected and modified: Q:\src\msbuild\src\MSBuild.UnitTests\MSBuildServer_Tests.cs.
  • Target method: BuildsWhileBuildIsRunningOnServer() (starts around line 177).
  • Original risky code: watcher Created handler directly called mre.Set(); (line 198 in stack trace context).
  • Change 1: wrapped mre.Set() in try { ... } catch (ObjectDisposedException) { ... } with comment explaining benign late/duplicate macOS callbacks.
  • Change 2: inserted watcher.EnableRaisingEvents = false; before _env.Dispose(); in cleanup to reduce callback race window.
  • Net diff from git: 1 file changed, 11 insertions(+), 1 deletion(-).]

[Progress Assessment:

  • Completed: root-cause analysis, code hardening implemented in test file, method verification performed.
  • Completed: rationale communicated to user.
  • In-progress/blocked: draft PR creation workflow (branch/commit/push/PR) started but interrupted before completion.
  • Not completed: no push, no remote draft PR URL, no evidence of local commit success after interruption.]

[Context Validation:

  • Critical context preserved: exact failing test/method, exception type/location, root cause (dispose order + async callbacks), exact code edits, and current git/PR state.
  • Remaining continuation needs: re-run git branch/commit sequence, push branch, open draft PR, optionally run targeted tests before PR.]

[Recent Commands Analysis:

  • Last agent commands (exact tools/actions):
    1. functions.report_intent with intent "Creating draft PR".
    2. functions.powershell command: cd Q:\src\msbuild && git --no-pager diff --stat && echo "---" && git --no-pager branch --show-current
      • Result: showed modified file stat only: src/MSBuild.UnitTests/MSBuildServer_Tests.cs | 12 +++++++++++-, 1 file changed, 11 insertions(+), 1 deletion(-), exit code 0; branch name not shown (likely detached HEAD).
    3. functions.powershell command: cd Q:\src\msbuild && git --no-pager branch --show-current && git --no-pager log --oneline -1
      • Result: ac211928a0 (HEAD, origin/main, origin/HEAD) Only get command line args names on modern .NET (#13314); exit code 0.
    4. functions.powershell command attempting branch+commit:
      git checkout -b fix/server-test-disposed-handle && git add ... && git commit -m "Fix ObjectDisposedException crash ... Fixes #13175 ..."
      • Result: "The execution of this tool, or a previous tool was interrupted."
  • Immediate pre-summarization state: agent was actively executing git steps to create a branch and commit for a draft PR in response to user request; process interrupted before completion.
  • Triggering context: user requested a structured conversation summary emphasizing the most recent agent tool commands/results after interrupted PR-prep ope...

Created from Copilot CLI via the copilot delegate command.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@dotnet-policy-service
Copy link
Contributor

Hello @@copilot, I noticed that you’re changing an .swr file or any file under src/Package/MSBuild.VSSetup.. Please make sure to validate this change by an experimental VS insertion. This is accomplished by pushing to an exp/* branch, which requires write permissions to this repo.

…ildsWhileBuildIsRunningOnServer

Co-authored-by: rainersigwald <3347530+rainersigwald@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix disposal order for watcher and mre in tests Fix ObjectDisposedException in BuildsWhileBuildIsRunningOnServer test Mar 2, 2026
…e reordering

Co-authored-by: rainersigwald <3347530+rainersigwald@users.noreply.github.com>
@rainersigwald rainersigwald marked this pull request as ready for review March 2, 2026 20:37
Copilot AI review requested due to automatic review settings March 2, 2026 20:37
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes an ObjectDisposedException crash in the BuildsWhileBuildIsRunningOnServer unit test by ensuring FileSystemWatcher is disposed before the ManualResetEvent it signals, preventing late callbacks from using a disposed handle.

Changes:

  • Reordered mre and FileSystemWatcher using declarations so watcher is disposed first.
  • Added an explanatory comment to prevent accidental reordering in the future.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@rainersigwald rainersigwald enabled auto-merge (squash) March 2, 2026 20:57
@rainersigwald rainersigwald merged commit 4b59b45 into main Mar 3, 2026
10 checks passed
@rainersigwald rainersigwald deleted the copilot/fix-disposal-order-issue branch March 3, 2026 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants